Anchor PS replica service lookup in the nightly CodeceptJS test - #1200
Conversation
getServiceDetailsByPartialDetails matches with String.includes and returns the first hit, so the '_2' lookup in pmm_ps_replica_integration_test.js also matches the source node whenever the setup's random service-name suffix starts with 2 (ps_pmm_replication_8_0_1_20753 contains '_2'). PMM-T2029 then opened the source's MySQL Replication Summary dashboard while asserting the replica's expectations, and failed with 'Expected 4 Elements without data but found 7' -- every empty panel a replica-only one. Add getServiceDetailsByRegexAndParameters, mirroring the Playwright suite's helper of the same name, and select both nodes with an anchored pattern. The helper requires exactly one match rather than taking the first, so an ambiguous pattern fails with the candidates listed instead of silently asserting against the wrong instance. Signed-off-by: travagliad <215686151+travagliad@users.noreply.github.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan includes up to 1 review per rolling hour; 0 remain after this review. WalkthroughThe change adds a regex- and parameter-based inventory lookup helper. Replica integration scenarios use anchored service-name matching with replication-set filtering for the current replica and the ChangesReplica service lookup
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Review feedback: the test file should not define helpers. Drop serviceNameRegex and hardcode the anchored pattern at both call sites. The produced patterns are unchanged. Signed-off-by: travagliad <215686151+travagliad@users.noreply.github.com>
Review feedback: this should be as small as PR #1178. getServiceDetailsByRegex already exists in the CodeceptJS inventoryAPI, and anchoring the pattern on ^ps_pmm_replication_ already scopes the lookup to the replication setup (gr/single/mysql use different container prefixes), so the replication_set filter it dropped was redundant. Revert inventoryAPI.js entirely. Signed-off-by: travagliad <215686151+travagliad@users.noreply.github.com>
Failures fixed (investigator)
Nightly E2E tests Matrix (remote PMM Server), jobtest execution / @nightly(95723345960)codeceptjs-e2e/tests/qa-integration/pmm_ps_replica_integration_test.js/@pmm-ps-replica-integration @not-ui-pipeline @nightly— PMM-T2029 "Verify dashboard for PS Replica Instance", data case{"serviceName":"_2"}Our own test code, not a PMM regression. Same bug as #1178 fixed in the Playwright
suite; that PR only touches
e2e_tests/**, so the CodeceptJS copy stayed broken.What failed
Note the URL: the data case is
_2(the replica), but the dashboard opened isps_pmm_replication_8_0_1_20753— the source. Every empty panel is areplica-status panel, empty because the source legitimately has no replica state.
The
_1case in the same run passed.Root cause — a substring match on the node index
percona-server-setup.yml:118appends a random suffix to both service names(
random_service_name_value: "_{{ 99999 | random + 1 }}"), so the pair isps_pmm_replication_8_0_1_20753/ps_pmm_replication_8_0_2_20753.The test selected each node with
getServiceDetailsByPartialDetails({ service_name: '_2', … }),and that helper matches with
String.includesand returns the first hit:'ps_pmm_replication_8_0_1_20753'.includes('_2')istrue— the random suffix_20753starts with2. So whenever the suffix starts with the other node'sindex and that service sorts first in
v1/management/services(ordered by therandom
service_idUUID), the lookup silently returns the wrong instance.That is why this is intermittent rather than permanently red: it needs both the
suffix collision (~11% of suffixes) and the unlucky ordering (~50%).
The fix
Same shape as #1178 — swap the lookup for the already-existing
getServiceDetailsByRegexwith an anchored pattern^ps_pmm_replication_.*_N(_\d+)?$, which tolerates the random suffix beingpresent or absent but cannot run past the node index. Two lines, no new helper.
The dropped
replication_set: 'ps-async-replication'argument is redundant oncethe pattern is anchored: the setups use distinct container prefixes
(
ps_pmm_replication_vsps_pmm_gr_/ps_pmm_/mysql_pmm_), so^ps_pmm_replication_already scopes the lookup to this setup. Checked against ashared-server inventory containing all of those — exactly one match per node.
getServiceDetailsByPartialDetailsitself is left untouched; its other callers(
details_explain_test.js, the MongoDB dashboard tests) rely on substringsemantics.
Reproduction
Throwaway Linode VM, PMM Server
perconalab/pmm-server:3.9.1-rc(digest
sha256:003f9c25…, the same image the failing run used), clientpmm3-rc,--database ps,SETUP_TYPE=replication. Services re-registered with the failingrun's exact names (
…_20753) and re-rolled until the API returned_1first —i.e. the exact CI condition:
Running the unmodified test against that state reproduced the failure exactly,
including the wrong service in the URL:
(9 empty panels here vs 7 in CI — this VM is younger, so two
Last 24 hourspanels are also still empty. The defect is identical: the source's dashboard
opened for the replica's assertions.)
Verification
Same VM, same colliding service names, same API ordering that produced the
failure, with the branch synced onto it:
npx eslintclean on the VM (repo's own pinned config, exit 0).Caveat: that VM run verified an earlier revision of this branch, which used a
new
getServiceDetailsByRegexAndParametershelper. Simplifying to the existinggetServiceDetailsByRegexhappened after teardown, so it has not been re-run ona live server. The regex strings are byte-identical between the two revisions,
and the dropped
replication_setfilter was verified redundant against anightly-like inventory, but the end-to-end green run predates the simplification.
Not fixed here — PMM-T1142
The same nightly also failed
codeceptjs-e2e/tests/QAN/timerange_test.js/@qan— PMM-T1142,.selected-overview-rownever visible after opening thecopied QAN link (job 95723345803).
Unrelated to this change and not addressed here: it passed on the previous
nightly and did not reproduce (3/3 green on the VM). For whoever picks it up —
the trace shows the selected query was present in the post-reload response and
briefly carried the highlight class before losing it, which points at QAN's own
selection state rather than a test selector. Deliberately not papered over.